Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix password hint check #5189

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Conversation

stefan0xC
Copy link
Contributor

@stefan0xC stefan0xC commented Nov 12, 2024

as noticed by @jjlin #5179 (comment) this is currently not correct.

don't show password hints if you have disabled the hints with PASSWORD_HINTS_ALLOWED=false or if you have not configured mail and also not opted into showing password hints

don't show password hints if you have disabled the hints with
PASSWORD_HINTS_ALLOWED=false or if you have not configured mail and
opted into showing password hints
@jjlin
Copy link
Contributor

jjlin commented Nov 12, 2024

I'm ambivalent about this change, but if this will be the new behavior, then I think the descriptions in config.rs and .env.template should be updated accordingly.

@dani-garcia
Copy link
Owner

Change looks good to me, I think we should we also mention in the docs that password_hints_allowed needs to be enabled for show_password_hint to work, should be obvious, but might as well be explicit about all the interactions between options.

@dani-garcia dani-garcia merged commit adb21d5 into dani-garcia:main Nov 12, 2024
5 checks passed
@dani-garcia
Copy link
Owner

Thanks!

Copy link
Collaborator

@BlackDex BlackDex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better indeed. Thanks.

@stefan0xC stefan0xC deleted the fix-password-hint branch November 12, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants